Improvements and fixes to NetCDFWriter
#4890
Conversation
ext/OceananigansNCDatasetsExt.jl
Outdated
|
|
||
| function squeeze_reduced_dimensions(data, reduced_dims) | ||
| # Fill missing indices from the right with 1s | ||
| indices = Any[:,:,:] |
There was a problem hiding this comment.
| indices = Any[:,:,:] | |
| indices = Any[:, : ,:] |
what does this mean exactly?
There was a problem hiding this comment.
The comment is meant for the whole algorithm, not just that line. Basically it gets Fields that have Nothing in any of the locations and it gets rid of that dimension. (I'm calling these reduced_dims but there's probably a better name.)
So for example, if data comes from an array that has loc = Center, Center, Nothing, it'll reduce its parent data by indexing it as data[:, :, 1]; i.e. a 2D array in this case.
I'm testing this out (hence a draft PR) after some discussions with @ali-ramadhan. Basically we want to keep all NetCDF arrays 3D (to mimic Oceananigans' behavior) as much as possible, but in NetCDF there's no concept of location, so we can't easily pass LZ = Nothing to NetCDF. In these cases it probably makes sense to squeeze that dimension (or whatever the verb for this is) so that we know that dimension is reduced.
There was a problem hiding this comment.
Can you add attributes that indicate the location? We can add custom attributes to data even if they are not built in, right?
There was a problem hiding this comment.
I can (and in fact I probably will do it regardless), but that doesn't affect calculations with any of the software (that I'm aware of) which reads NetCDF. So broadcasting and other useful things wouldn't work. Without the singleton dimensions (i.e. with the change that I'm exploring here), those things will work out of the box for most software.
Also if we include the singleton reduced dimension, we still would need to pick a value for that length-1 coordinate. In Oceananigans, that value is nothing I think, but there's no analog to that for NetCDF, so we'd need to pick an arbitrary value like 1 or 0, which I'd like to avoid.
There was a problem hiding this comment.
Also to be clear, this PR would move the NetCDF behavior closer to Oceananigans' behavior. Currently the NetCDF writer also drops singleton dimensions whenever we slice a field, even though that sliced dimension (or length 1) has a proper value. This PR would change the behavior and keep that sliced length-1 dimension in the NetCDF, just like Oceananigans does.
| # At the moment, this returns the full nodes even when the field is sliced. | ||
| # https://github.com/CliMA/Oceananigans.jl/pull/4814 will fix this in the future. | ||
| node_data = nodes(field; with_halos) |
There was a problem hiding this comment.
Note to future self: revisit this after #4814 is merged. It should make this line work out of the box and tests pass
Contains a few improvements that will make #4848 easier. Also a couple of fixes for edge cases.